Refactor dpnp includes and add extension example#2941
Conversation
Enables find_package(Dpnp) out of the box
move dpnp4pybind11.hpp and usm_ndarray_constants.h into a top-level include directory, similar to original dpctl layout
makes include explicit
serves to test dpnp cmake integration
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/2941/index.html |
|
Array API standard conformance tests for dpnp=0.21.0dev0=py313h509198e_82 ran successfully. |
622aa32 to
c735f53
Compare
cae3b56 to
020e5ea
Compare
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
LGTM
Thank you @ndgrigorian
A few minor comments below:
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Great job!
No more comments from my side
LGTM, thank you @ndgrigorian
| help="Path to dpnp libtensor include directory.", | ||
| ) | ||
| parser.add_argument( | ||
| "--cmakedir", |
There was a problem hiding this comment.
Would it make sense to test --cmakedir / Dpnp_ROOT in the example also?
| # | ||
|
|
||
| if(NOT Dpnp_FOUND) | ||
| find_package(Python 3.10 REQUIRED COMPONENTS Interpreter Development.Module) |
There was a problem hiding this comment.
Would it make sense to have upper limit also?
| find_package(Python 3.10 REQUIRED COMPONENTS Interpreter Development.Module) | |
| find_package(Python 3.10...<3.15 REQUIRED COMPONENTS Interpreter Development.Module) |
| dpnp/**/*.pyd | ||
| *~ | ||
| core | ||
| *.so |
There was a problem hiding this comment.
We have a pattern with ...*so already above. Do we need to be more specific here and to ignore only ones generated in example folder?
|
|
||
| - name: Install example requirements | ||
| run: | | ||
| mamba install pytest dpcpp_linux-64">=2026.0.0" cmake ninja scikit-build ${{ env.TEST_CHANNELS }} |
There was a problem hiding this comment.
As an option we can add a dedicated env file to dpnp/enviroments folder and install all the requirements in Setup miniconda step
| Dpnp_INCLUDE_DIR | ||
| dpnp4pybind11.hpp | ||
| PATHS "${_dpnp_include_dir}" "${Python_INCLUDE_DIRS}" | ||
| PATH_SUFFIXES dpnp/include |
There was a problem hiding this comment.
Do we need to use NO_DEFAULT_PATH here ?
| install( | ||
| FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake | ||
| DESTINATION dpnp/resources/cmake | ||
| ) | ||
| install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION lib/cmake/dpnp) |
There was a problem hiding this comment.
We can reduce duplication:
| install( | |
| FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake | |
| DESTINATION dpnp/resources/cmake | |
| ) | |
| install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION lib/cmake/dpnp) | |
| foreach(_dest IN ITEMS dpnp/resources/cmake lib/cmake/dpnp) | |
| install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION ${_dest}) | |
| endforeach() |
|
|
||
| # install dpnp4pybind11.hpp and usm_ndarray_constants.h into include folder | ||
| install( | ||
| DIRECTORY ${CMAKE_SOURCE_DIR}/dpnp/include/ |
There was a problem hiding this comment.
It actually means the recurses into ALL subdirs. But there is another rule at dpnp/CMakeLists.txt:147-156 which installs the Cython-generated headers from the build.
Thus the rule will grab dpnp/tensor/_usmarray.h and _usmarray_api.h and install them to <prefix>/dpnp/include/dpnp/tensor. Exactly the same as rules at dpnp/CMakeLists.txt:147-156 do.
So the same files might be written twice to the same location.
There was a problem hiding this comment.
We might need to specify explicitly:
install(FILES
${CMAKE_SOURCE_DIR}/dpnp/include/dpnp4pybind11.hpp
${CMAKE_SOURCE_DIR}/dpnp/include/usm_ndarray_constants.h
DESTINATION dpnp/include)or to add PATTERN "dpnp/tensor" EXCLUDE to the end of install command.
This PR proposes a refactor of dpnp's includes and the introduction of an example extension, to be tested to make sure examples using dpnp and dpnp.tensor can be written.
The
usm_ndarray_constants.hheader anddpnp4pybind11.hppare moved todpnp/include, removing a relative include and aligning with previous dpctl behavior.Also introduces dpnp-config.cmake, which enables
find_package(Dpnp)out of the box, which is installed with dpnp, and--cmakedircommand line option, which leads to the CMake config.